Skip to content

docs: ADR-0003 extension architecture#110

Open
mrbrandao wants to merge 3 commits into
LobsterTrap:mainfrom
mrbrandao:adr/extension-architecture
Open

docs: ADR-0003 extension architecture#110
mrbrandao wants to merge 3 commits into
LobsterTrap:mainfrom
mrbrandao:adr/extension-architecture

Conversation

@mrbrandao

@mrbrandao mrbrandao commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add ADR-0003 documenting the extension architecture decision: 5 kinds (target, repo, runtime, source, scan), YAML manifests, stdin/stdout protocol with gRPC as future option
  • Add paired design doc with kind taxonomy Mermaid diagram, discovery flow, manifest schema, and hello world examples (bash target, Python repo extension)

Related Issues

Part of the Lola Go migration and extension platform architecture initiative.

Test Plan

  • ADR follows the project template format
  • Design doc Mermaid diagrams render correctly
  • Hello world extension examples are self-contained and runnable

AI Disclosure

AI-assisted with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added architecture decision record documenting the extension system design, including extension types, YAML manifest format, and communication protocol specifications.

Review Change Stack

@mrbrandao mrbrandao added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 29, 2026
@coderabbitai

coderabbitai Bot commented Apr 29, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f90575f1-9c40-4f88-9d88-116d4ec9cc97

📥 Commits

Reviewing files that changed from the base of the PR and between bcc5cec and 360bcea.

📒 Files selected for processing (1)
  • docs/adr/extension-architecture.md
✅ Files skipped from review due to trivial changes (1)
  • docs/adr/extension-architecture.md

📝 Walkthrough

Walkthrough

This PR adds a new Architecture Decision Record proposing a formal extension system for Lola. The ADR defines five extension kinds, specifies a YAML manifest plus language-agnostic protocol approach, documents rationale and consequences, and lists considered alternatives.

Changes

Extension Architecture Design

Layer / File(s) Summary
Extension architecture design decision record
docs/adr/extension-architecture.md
Proposes a formal extension system with five kinds (target, repo, runtime, source, scan), YAML manifest format, and language-agnostic communication protocol, documenting rationale, consequences (process isolation, protocol versioning), and alternatives (hardcoded handlers, shared libraries, gRPC).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • LobsterTrap/lola#108: Adds a "Design Documents" section to organize architectural documentation alongside this extension system ADR.

Suggested reviewers

  • SecKatie
  • sergio-correia

Poem

🐇 A blueprint for builders, both near and afar,
Extensions now structured by YAML and char,
Five kinds in the garden, where protocols bloom,
Process isolation keeps chaos from the room!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'docs: ADR-0003 extension architecture' directly and clearly summarizes the main change—a new architecture decision record documenting the extension system design.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
docs/dev-guide/design/extension-architecture.md (2)

162-163: Add error handling for malformed JSON input.

The script parses stdin without handling potential JSON decode errors. While this is a "hello world" example, adding basic error handling would make it more robust and instructive.

♻️ Suggested enhancement
-request = json.loads(sys.stdin.read())
-action = request["action"]
+try:
+    request = json.loads(sys.stdin.read())
+    action = request["action"]
+except (json.JSONDecodeError, KeyError) as e:
+    print(json.dumps({"error": str(e)}), file=sys.stderr)
+    sys.exit(1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/dev-guide/design/extension-architecture.md` around lines 162 - 163, The
code reads stdin and calls json.loads(sys.stdin.read()) then accesses
request["action"] without handling malformed input; update the block around
json.loads and the access to request["action"] (the request variable and action
lookup) to catch json.JSONDecodeError and KeyError (and optionally generic
Exception), emit a clear error to stderr (or process logger) and exit with a
non-zero status, and only proceed to use action when the JSON was parsed and the
"action" key exists.

93-94: Consider noting the jq dependency.

The script relies on jq for JSON parsing but doesn't document this dependency. For a "hello world" example meant to be runnable, noting this requirement would help users.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/dev-guide/design/extension-architecture.md` around lines 93 - 94, The
example uses jq in the line action=$(echo "$input" | jq -r '.action') but the
document doesn't mention jq as a prerequisite; update the text around the code
snippet to state that jq is required (or modify the example to check for jq at
runtime and print a friendly error if missing), and reference the two shell
lines (input=$(cat) and action=$(echo "$input" | jq -r '.action')) so readers
know exactly where the dependency is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/dev-guide/design/extension-architecture.md`:
- Around line 149-180: Add a brief setup note to the extension-architecture docs
that instructs users to make the example script my-catalog.py executable and to
ensure it contains the proper shebang (#!/usr/bin/env python3); specifically,
mention running chmod +x on the script and verifying the shebang line so the
script can be run directly as an executable.
- Around line 134-138: The fenced code block showing the directory structure
(the block starting with "~/.lola/extensions/my-catalog/") lacks a language
identifier; add "text" after the opening triple backticks so it becomes ```text
to improve rendering/tooling (update the fenced block that contains
extension.yaml and my-catalog.py).
- Around line 193-195: The fenced code block containing the protocol diagram
(the line starting with "Core → [write request to stdin] → Extension process →
[read response from stdout] → Core") lacks a language specifier; update the
opening fence from ``` to ```text so the block is rendered consistently (i.e.,
change the code fence for that protocol diagram to use the `text` language
identifier).
- Around line 71-75: Update the fenced code block showing the directory tree in
extension-architecture.md to include a language specifier (e.g., change the
opening ``` to ```text) so the block is rendered and highlighted correctly;
locate the block containing "~/.lola/extensions/hello-target/" and modify its
fence to use the "text" language identifier.
- Around line 170-176: The error branch in the resolve action prints an error
JSON but still exits with code 0; update the resolve handling (the branch where
action == "resolve" that looks up name in CATALOG and prints the error JSON) to
call sys.exit(1) after printing the error so the process returns a non-zero exit
code on failure; ensure sys is imported at the top if not already (reference the
resolve branch and the CATALOG/name lookup to locate the change).
- Around line 88-121: Add a brief "Setup" note immediately after the
hello-target.sh example that tells users to make the script executable before
running it; explicitly state they should set executable permissions on the
installed extension script (e.g., use chmod +x on the hello-target.sh /
installed extension file) so they don't hit "permission denied" errors when
invoking the script.

---

Nitpick comments:
In `@docs/dev-guide/design/extension-architecture.md`:
- Around line 162-163: The code reads stdin and calls
json.loads(sys.stdin.read()) then accesses request["action"] without handling
malformed input; update the block around json.loads and the access to
request["action"] (the request variable and action lookup) to catch
json.JSONDecodeError and KeyError (and optionally generic Exception), emit a
clear error to stderr (or process logger) and exit with a non-zero status, and
only proceed to use action when the JSON was parsed and the "action" key exists.
- Around line 93-94: The example uses jq in the line action=$(echo "$input" | jq
-r '.action') but the document doesn't mention jq as a prerequisite; update the
text around the code snippet to state that jq is required (or modify the example
to check for jq at runtime and print a friendly error if missing), and reference
the two shell lines (input=$(cat) and action=$(echo "$input" | jq -r '.action'))
so readers know exactly where the dependency is used.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 610836da-0f5c-4351-b75c-168e26d99fa5

📥 Commits

Reviewing files that changed from the base of the PR and between a343c6e and 6469d92.

📒 Files selected for processing (2)
  • docs/adr/0003-extension-architecture.md
  • docs/dev-guide/design/extension-architecture.md

Comment thread docs/dev-guide/design/extension-architecture.md Outdated
Comment thread docs/dev-guide/design/extension-architecture.md Outdated
Comment thread docs/dev-guide/design/extension-architecture.md Outdated
Comment thread docs/dev-guide/design/extension-architecture.md Outdated
Comment thread docs/dev-guide/design/extension-architecture.md Outdated
Comment thread docs/dev-guide/design/extension-architecture.md Outdated
@mrbrandao

Copy link
Copy Markdown
Collaborator Author

@SecKatie @sergio-correia, this is also good to go, check out

@SecKatie

Copy link
Copy Markdown
Collaborator

@mrbrandao can we agree to a shorter ADR that announces our desire for extensions and the inspirations we want to take from, but doesn't yet design all the features? I was thinking that we should work on the design doc after we have a better idea of the shape our golang project will take and can sit down and whiteboard.

mrbrandao and others added 3 commits May 29, 2026 11:55
Add Architecture Decision Record for the extension
system with 5 kinds (target, repo, runtime, source,
scan), YAML manifests, and language-agnostic protocol.

Design doc includes kind taxonomy diagram, discovery
flow, manifest schema, and hello world examples.

Co-Authored-By: Claude Sonnet <noreply@anthropic.com>
- Add `text` language specifier to three bare fenced code blocks (MD040)
- Document `jq` prerequisite for bash target example
- Add `chmod +x` setup notes for both hello world examples
- Add try/except error handling for malformed JSON input in Python example
- Exit with code 1 when resolve action finds no matching module

Co-Authored-By: Claude <noreply@anthropic.com>
@mrbrandao mrbrandao force-pushed the adr/extension-architecture branch from bcc5cec to 360bcea Compare May 29, 2026 14:56
@mrbrandao

Copy link
Copy Markdown
Collaborator Author

@mrbrandao can we agree to a shorter ADR that announces our desire for extensions and the inspirations we want to take from, but doesn't yet design all the features? I was thinking that we should work on the design doc after we have a better idea of the shape our golang project will take and can sit down and whiteboard.

@SecKatie yes, agree! Adjusted check out again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants